fix(sync): install pg_stat_statements into a named schema and verify through the resolver - #237
fix(sync): install pg_stat_statements into a named schema and verify through the resolver#237veksen wants to merge 3 commits into
Conversation
`installPgStatStatements()` issued a bare `CREATE EXTENSION`, which lands in `public`. The extension owns views there that a migration tool reconciling `public` cannot drop, so the tool aborts half-applied with SQLSTATE 2BP01 and the failure surfaces far from the extension. The install now takes a schema, defaulting to `query_doctor`, and the verify probe resolves the schema through `getQuerySource()` instead of reading an unqualified name through the `search_path` — which returns 42P01 for exactly the placement the install now produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The verify step read `getQuerySource()`, which resolves either `pg_stat_statements` or `pg_stat_monitor` with no ordering, so with both installed it could verify an extension the install never touched. It now resolves the one extension it installed. The public-schema warning was guarded by an instance flag, but `ConnectionManager.getConnectorFor` builds a new connector on every poll, so it fired every ten seconds. The guard is now keyed by connection: once per database, and still once per database when several are attached. `POST /postgres/extensions/pg_stat_statements` takes the schema through the websocket controller as well, and rejects an empty one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.

Query Doctor — 6 successful checks
More details via MCP → get_ci_run({ runId: "01a045d7-c1ca-7717-abbf-2d072097848f" }) · view run · docs
3 queries read against main on assumed statistics of 10,000,000 rows per table. Sync production stats for costs measured against your real data.
getRecentQueries and resetPgStatStatements matched the literal text `relation "pg_stat_statements" does not exist`. Every statement they send is schema-qualified, so Postgres names the schema in the message and the match never fired: a missing extension reached the caller as a generic PostgresError instead of ExtensionNotInstalledError, and the app showed raw SQL text rather than its install panel. Both now read the SQLSTATE — 42P01 for a missing relation, 42883 for a missing function, 3F000 when the schema itself is gone, which is what a qualified function call reports. getQuerySource also logs the extension and schema it resolved. A run that read the statistics and a run that found nothing were indistinguishable in the logs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Folded into Query-Doctor/Site#4205. The collector image builds from Site's apps/analyzer, so the fix belongs there; this PR was opened unasked. |
Goal
A user following the CI guide should be able to run their migrations afterwards. Today installing
pg_stat_statementsbreaks any project whose migration tool reconcilespublic, and the failure surfaces as unrelated broken tests. Closes Query-Doctor/Site#4199 on the analyzer side; the docs half and the other producers of the same state are in Query-Doctor/Site#4202.What
Before:
POST /postgres/extensions/pg_stat_statementsran a bareCREATE EXTENSION pg_stat_statements, which lands the extension inpublic. The extension ownspg_stat_statements_infothere and will not let a migration tool drop it, sodrizzle-kit push --forceaborts half-applied with SQLSTATE2BP01. Nothing at the point of failure names the extension.After: the endpoint creates a schema (
query_doctorby default, or theschemafield in the request body) and puts the extension in it. The response carries the schema back. An extension that was already installed stays where it is, and every read of apublic-schema extension logs what is wrong and theALTER EXTENSION ... SET SCHEMAthat fixes it.A missing extension also reaches the caller as
ExtensionNotInstalledErroragain, so the app shows its install panel instead of raw SQL error text. A resolve now logs the extension and schema it read, so a run that found the statistics can be told from one that found nothing.The two halves are coupled. The verify step used to probe an unqualified
pg_stat_statements, which resolves through thesearch_pathand returns42P01for exactly the placement the install now produces — shipping the install alone would report every successful install as a failure.How
Read
src/sync/pg-connector.tsfirst.installPgStatStatements()takes{ schema }, defaulting toPostgresConnector.EXTENSION_SCHEMA. The name goes throughPgIdentifier, so a caller-supplied schema is quoted rather than interpolated.The verify step resolves the schema from
pg_extension JOIN pg_namespacethrough the newgetExtensionSchema(), then probes the view qualified. That helper asks about one named extension rather than reusinggetQuerySource(), which matchespg_stat_statementsorpg_stat_monitorwith no ordering and could otherwise verify an extension this call never installed.warnIfExtensionIsInPublic()fires from the resolver, so apublicinstall is named at capture time rather than discovered through broken migrations. Its guard is aWeakMapkeyed by thePostgresconnection:ConnectionManager.getConnectorForbuilds a new connector on every poll, so an instance flag would warn every ten seconds, and a process-wide flag would silence every source database after the first.getRecentQueries()andresetPgStatStatements()classify errors by SQLSTATE through the newisMissingExtensionObject(). They matched the literalrelation "pg_stat_statements" does not exist, but every statement they send is schema-qualified, so Postgres names the schema in the message and the match never fired. The codes are42P01for a missing relation,42883for a missing function, and3F000when the schema itself is gone, which is what a qualified function call reports rather than42883.src/remote/remote.dto.tsaddsInstallPgStatStatementsRequest, which the HTTP route and the websocket controller both decode. An empty schema is a 400 rather than a 500.The default is
query_doctorbecause a migration tool reconciles the schemas it declares, and no tool declares that one. Any non-publicname works.Databases that already have the extension in
publicare not migrated.ALTER EXTENSION ... SET SCHEMAneeds an ownership the tool may not hold, and relocating a schema object someone else installed is not ours to decide; the warning tells them instead.Moving the extension trips the schema-drift gate once. The dump records the extension's
schemaNameand theschemaNameof the two views it owns, so a run that installs intoquery_doctordiffers from a baseline captured againstpublicand the gate reports "This PR changes the database schema" on a pull request containing no migration. Measured on a live instance, and reproduced here by diffing the dump against two servers:extensionsandviewsdiffer byschemaName, and nothing else does (theoiddifferences the diff also shows are whatschemaDigestalready ignores). It clears as soon as a run on the default branch writes a new baseline. Whether the drift comparison should ignore objects an extension owns is a separate question, not settled here.Tests
src/sync/pg-connector.test.tsadds four integration tests againstpostgres:17: a fresh install lands inquery_doctorand leaves nothing namedpg_stat_statements*inpublic; an explicit{ schema: "ext" }is honoured; an extension pre-installed inmonitoringverifies and reports that schema (this one fails with42P01without the resolver change); and apublicextension warns once across two connectors over the same connection.An integration test pins those SQLSTATEs against a real server and asserts the messages do not contain the text the old check looked for. A unit test covers the classifier, including the privilege and syntax codes it must not claim.
Full suite: 44 files, 451 tests, all passing.
tsc --noEmitclean.